Conversation
note that this cmsdist PR changes only |
Yes, the question of compiler flags is something I should open as a separate issue. In fact, I started to open a trackreco issue about it, but I don't even know what to call it... perhaps "Make standalone compiler options match what CMSSW is doing for -ffast-math or similar" - ? Anyway, I am bringing it up here simply because I don't like to rain teeny-tiny fixes for the mkFit standalone build into cms-sw. |
|
When all this first came up I did this on my overlap branch: I think change (bugfix) on line 64 is crucial ... I forgot to change it after renaming DictsDict to WriteMemFileDict (as there are now two dictionary files). |
|
Thanks @osschar, the bugfixes in the commit you mention were already included my PR #119 to this repo, which became cms-sw#40985, which was merged on March 9. Our trackreco fork is sufficiently up to date with respect to CMSSW, and we now have those fixes. This PR is different, it addresses a subtle |
|
Oh, I see you've made this change already ... sorry. |
This is in response to issue #121 from @slava77. I believe it takes care of the Makefile problem where putting 2 targets on the same line might result in the rule for those targets being run simultaneously by 2 threads (as @slava77 hypothesized in Skype). The issue seems to occur twice in the Makefile; this PR fixes both. It also addresses the incomplete cleanup issue, where a couple of
.ccsource files that are created byrootclingduringmakeget left behind bymake distclean. I tested the documented procedure inREADME_buildFromCMSSW.txt, and the new Makefile works both on a fresh clone and a directory tree cleaned bymake distclean. I leave it to @osschar to check on the goodness of my solution.If we decide to make this PR into a further PR to cms-sw, then I propose that we also consider changing the default compilation for standalone builds to
-Ofast -fno-reciprocal-math -mrecip=none, since this combination has been approved by CMSSW as not harming reproducibility between AMD and Intel platforms. See cms-sw/cmsdist#8280. This combination of options improves the performance of mkFit by quite a lot: my little test with InitialStep runs almost 20% faster when the above options are provided throughCXXUSERFLAGS. In general I think our default standalone build should match the default options for the full CMSSW build... but I can't tell if these flags are now the default for CMSSW builds. Can someone please comment on that?